Dispatcher Build Gate Summary (local)

• rubocop (diff-only): PASS (0 offenses)
• brakeman: skipped (per repo build gate config; disabled)
• tests (affected engines): PASS
  • core: PASS

Note: test output still prints BSON Symbol deprecation warning (expected to be addressed by WA-NEW-010 / PR #635).

Architecture Review

Verdict: PASS_WITH_NOTES (LOW)

Contained metrics-layer change; DST-safe week math via date-based calculations is the right direction.

Note

• core/app/models/workarea/metrics/by_week.rb:16 — Week boundary logic now exists in multiple places (scope + scoring). If definitions change later, logic could drift.
  • Suggestion: consider centralizing week boundary/week-diff helpers (small module/private method) to keep one definition.

Test Quality Review

Verdict: CHANGES_REQUIRED (MEDIUM)

The DST regression coverage added in test_weeks_ago is good, but there are still travel_to calls in this file that are not in block form and can leak frozen time across tests.

Findings

• core/test/models/workarea/metrics/product_by_week_test.rb:15 — travel_to in test_last_week is called without a block, so time may not be restored automatically.

  • Fix: use travel_to(...) do ... end (or ensure travel_back) so time state is always reset.

• core/test/models/workarea/metrics/product_by_week_test.rb:112 — Same issue in test_score.

  • Fix: convert to block form.

Once those are block-form, the suite should be much more robust against ordering/flakiness.

Fix applied for test-quality feedback

Pushed fix to branch wa-new-004-metrics-dst to address non-block-form travel_to calls leaking time across tests.

• Converted travel_to in test_last_week and test_score to block form.
• Verified: bundle exec ruby -Icore/test core/test/models/workarea/metrics/product_by_week_test.rb

Commit: c68b08d

Test Quality Review (re-run)

Verdict: PASS

Re-check after commit c68b08d6: all travel_to usages in core/test/models/workarea/metrics/product_by_week_test.rb are now block-form, so time state is restored and cross-test leakage risk is low.

Architecture Review (re-run)

Verdict: PASS

Re-check after c68b08d6: fix is test-only (block-form travel_to); no new architectural concerns introduced. Prior note about possibly centralizing week-boundary helpers remains optional.

Performance Review

Verdict: PASS

Performance risk is low. Date-based week boundaries keep the query as an indexed range scan on reporting_on and avoid DST drift; Ruby-side conversions are negligible.

✅ Review summary

Re-review after fix commit c68b08d6:

• Test quality: PASS
• Architecture: PASS
• Performance: PASS

Labeled merge:ready.

Performance Review

Verdict: PASS

Performance risk is low. Switching from second-based week math to date-based boundaries avoids DST drift and keeps DB queries as an indexed range scan on reporting_on. Ruby-side date conversions are negligible.

Architecture Review (re-run)

Verdict: PASS

Re-check after fix commit c68b08d6: test-only change (block-form travel_to), no new architectural concerns introduced. Optional previous note about centralizing week-boundary helpers remains non-blocking.

✅ Re-review status after c68b08d

• Architecture: PASS
• Performance: PASS

(Waiting on test-quality label if not yet updated.)

Labeled merge:ready.

✅ Re-review status after c68b08d

All requested follow-ups are satisfied and reviews are green:

• Test quality: PASS
• Architecture: PASS
• Performance: PASS

Labeled merge:ready.